-
Notifications
You must be signed in to change notification settings - Fork 73
Fix regional cache for the DOShardedTagCache #820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: a3e0889 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
return Math.max(...cachedValue.map((item) => item.time)); | ||
} | ||
// Otherwise we need to check the durable object on the ones that were not found in the cache | ||
const filteredTags = tags.filter((tag) => !cachedValue.some((item) => item.tag === tag)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const filteredTags = tags.filter((tag) => !cachedValue.some((item) => item.tag === tag)); | |
const filteredTags = deduplicatedTags.filter((tag) => !cachedValue.some((item) => item.tag === tag)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one nit
Thanks for the fix!
Could you please add a changeset |
Fix regional cache for the DOShardedTagCache.
I open it as a draft PR for now.
Fix #819
This implementation will store every single tag in their own cache entry.
I think we should add an option to the tag cache.It may make sense for some cases to store all the tag fromhasBeenRevalidated
in the same cache entry. It could be faster, but will likely ends up with less cache hit.After some thought, i don't think it's worth it, the only times where it could be useful would be if you don't use (or barely) the data cache, and that you have very few ISR/SSG cache entry. Maybe an improvement to do in another PR.